Skip to content

Zk name service#42

Merged
alainjobart merged 10 commits intovitessio:masterfrom
msolo:zk-name-service
Apr 17, 2014
Merged

Zk name service#42
alainjobart merged 10 commits intovitessio:masterfrom
msolo:zk-name-service

Conversation

@msolo
Copy link
Contributor

@msolo msolo commented Apr 16, 2014

No description provided.

@ryszard
Copy link
Contributor

ryszard commented Apr 16, 2014

The code itself looks good. However, we are trying to make the Vitess code compliant to the Go styleguide, which includes having doccomments of the form

// Foo foos the bar.
func Foo(bar Bar) {
...
}

In my experience this actually helps when you are trying to figure out what the code is doing. Can you please reword your doccomments in this spirit?

@msolo
Copy link
Contributor Author

msolo commented Apr 16, 2014

I'm surprised to hear you say that. Quickly scanning the diff, the vast majority of important functions and structures (particularly the public functions) are quite well documented.

Did you have a specific example where something in here needed more explanation? About the only example I can think of is the Election() method which is explained by comments on the structure itself.

alainjobart added a commit that referenced this pull request Apr 17, 2014
@alainjobart alainjobart merged commit 49fff35 into vitessio:master Apr 17, 2014
@msolo msolo deleted the zk-name-service branch April 17, 2014 17:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants